Fix a couple AST mistakes caught while porting the parser to Rust #47
Conversation
fluent-syntax/src/ast.js
Outdated
| @@ -210,13 +210,6 @@ export class Section extends Entry { | |||
| } | |||
| } | |||
|
|
|||
| export class Function extends Identifier { | |||
There was a problem hiding this comment.
Removing this doesn't look right. Function is a valid ASDL production: https://github.com/projectfluent/fluent/blob/a9fb740ecbf9326a484b812800f62a338a43f7da/spec/fluent.asdl#L72
Instead, I think when we create a CallExpression, we should ensure that the callee field is a Function, not just an Identifier:
fluent.js/fluent-syntax/src/parser.js
Line 569 in 3e8066c
| @@ -212,7 +212,7 @@ export default class FluentParser { | |||
| tags = this.getTags(ps); | |||
| } | |||
|
|
|||
| if (pattern === undefined && attrs === undefined && tags === undefined) { | |||
There was a problem hiding this comment.
Why are you removing the tags check here?
There was a problem hiding this comment.
My understanding was that we don't accept messages with just tags (they have to have attrs or value). Am I wrong?
There was a problem hiding this comment.
No, you're right! That's a good point. The EBNF confirms this: https://github.com/projectfluent/fluent/blob/master/spec/fluent.ebnf#L43. Thanks for correcting me.
|
updated to your feedback! |
fluent-syntax/src/parser.js
Outdated
| @@ -566,7 +566,10 @@ export default class FluentParser { | |||
|
|
|||
| ps.expectChar(')'); | |||
|
|
|||
| return new AST.CallExpression(literal.id, args); | |||
| return new AST.CallExpression( | |||
| new AST.Function(literal.id.name), | |||
There was a problem hiding this comment.
Can you add an assertion to make sure literal.id.name conforms to the EBNF? It should only accept [A-Z_?-]+.
There was a problem hiding this comment.
how would you like the assertion to look like? A regexp?
|
Re-requesting review because of the added check in the parser and test. I decided not to add |
|
@zbraniecki Would you like to port this to python-fluent? It would help me finish projectfluent/python-fluent#17. |
Port projectfluent/fluent.js#47 to python committer: GitHub <[email protected]>
Minor improvements to the parser and aligning error messages with the actual code. Found during porting to Rust.